[tcgc] fix path in override#4644
Merged
Merged
Conversation
|
All changed packages have been documented.
Show changes
|
commit: |
Contributor
⚡ Benchmark Results
Full details – comparing
|
| Metric | Baseline | Current | Change |
|---|---|---|---|
| total | 🟡 307.8ms | 🔴 616.9ms | +100.4% 🔴 |
| loader | 🟢 98.5ms | 🟢 158.2ms | +60.6% 🔴 |
| resolver | 🟢 12.0ms | 🟢 19.5ms | +62.7% 🔴 |
| checker | 🟢 101.5ms | 🟢 185.5ms | +82.7% 🔴 |
| validation | 🟢 24.4ms | 🟢 42.7ms | +75.2% 🔴 |
| ↳ validation/@azure-tools/typespec-azure-core | 🟢 3.4ms | 🟢 6.3ms | +84.3% 🔴 |
| ↳ validation/@typespec/http | 🟢 2.8ms | 🟢 5.5ms | +95.7% 🔴 |
| ↳ validation/@typespec/rest | 🟢 0.4ms | 🟢 0.6ms | +79.0% |
| ↳ validation/@typespec/versioning | 🟡 16.6ms | 🔴 28.4ms | +71.1% 🔴 |
| ↳ validation/compiler | 🟢 1.0ms | 🟢 1.7ms | +62.1% |
| linter | 🟢 70.5ms | 🟢 133.1ms | +88.8% 🔴 |
| ↳ linter/@azure-tools/typespec-azure-core/auth-required | 🟢 0.0ms | 🟢 0.0ms | +52.6% |
| ↳ linter/@azure-tools/typespec-azure-core/bad-record-type | 🟢 0.1ms | 🟢 0.2ms | +77.6% |
| ↳ linter/@azure-tools/typespec-azure-core/byos | 🟢 3.1ms | 🟢 5.7ms | +85.6% 🔴 |
| ↳ linter/@azure-tools/typespec-azure-core/casing-style | 🟢 0.3ms | 🟢 0.6ms | +83.2% |
| ↳ linter/@azure-tools/typespec-azure-core/composition-over-inheritance | 🟢 0.0ms | 🟢 0.1ms | +28.9% |
| ↳ linter/@azure-tools/typespec-azure-core/documentation-required | 🟢 0.5ms | 🟢 0.9ms | +74.2% |
| ↳ linter/@azure-tools/typespec-azure-core/friendly-name | 🟢 0.3ms | 🟢 0.6ms | +81.5% |
| ↳ linter/@azure-tools/typespec-azure-core/key-visibility-required | 🟢 0.1ms | 🟢 0.2ms | +91.3% |
| ↳ linter/@azure-tools/typespec-azure-core/known-encoding | 🟢 0.2ms | 🟢 0.3ms | +75.0% |
| ↳ linter/@azure-tools/typespec-azure-core/long-running-polling-operation-required | 🟢 0.1ms | 🟢 0.3ms | +95.7% |
| ↳ linter/@azure-tools/typespec-azure-core/no-case-mismatch | 🟢 0.1ms | 🟢 0.2ms | +79.9% |
| ↳ linter/@azure-tools/typespec-azure-core/no-closed-literal-union | 🟢 0.1ms | 🟢 0.3ms | +89.3% |
| ↳ linter/@azure-tools/typespec-azure-core/no-enum | 🟢 0.0ms | 🟢 0.0ms | +57.7% |
| ↳ linter/@azure-tools/typespec-azure-core/no-error-status-codes | 🟢 0.1ms | 🟢 0.1ms | +91.5% |
| ↳ linter/@azure-tools/typespec-azure-core/no-explicit-routes-resource-ops | 🟢 0.1ms | 🟢 0.1ms | +88.8% |
| ↳ linter/@azure-tools/typespec-azure-core/no-format | 🟢 0.3ms | 🟢 0.6ms | +87.3% |
| ↳ linter/@azure-tools/typespec-azure-core/no-generic-numeric | 🟢 0.3ms | 🟢 0.5ms | +63.3% |
| ↳ linter/@azure-tools/typespec-azure-core/no-header-explode | 🟢 9.2ms | 🟡 19.1ms | +106.6% 🔴 |
| ↳ linter/@azure-tools/typespec-azure-core/no-legacy-usage | 🟢 0.6ms | 🟢 1.1ms | +87.0% |
| ↳ linter/@azure-tools/typespec-azure-core/no-multiple-discriminator | 🟢 0.0ms | 🟢 0.1ms | +95.9% |
| ↳ linter/@azure-tools/typespec-azure-core/no-nullable | 🟢 0.1ms | 🟢 0.2ms | +87.8% |
| ↳ linter/@azure-tools/typespec-azure-core/no-offsetdatetime | 🟢 0.7ms | 🟢 1.2ms | +70.5% |
| ↳ linter/@azure-tools/typespec-azure-core/no-openapi | 🟢 1.2ms | 🟢 2.1ms | +78.5% |
| ↳ linter/@azure-tools/typespec-azure-core/no-private-usage | 🟢 1.0ms | 🟢 2.1ms | +99.9% 🔴 |
| ↳ linter/@azure-tools/typespec-azure-core/no-query-explode | 🟡 10.5ms | 🔴 20.0ms | +91.1% 🔴 |
| ↳ linter/@azure-tools/typespec-azure-core/no-response-body | 🟡 13.3ms | 🔴 23.0ms | +73.3% 🔴 |
| ↳ linter/@azure-tools/typespec-azure-core/no-rest-library-interfaces | 🟢 0.0ms | 🟢 0.0ms | +68.9% |
| ↳ linter/@azure-tools/typespec-azure-core/no-route-parameter-name-mismatch | 🟢 2.3ms | 🟢 5.2ms | +121.9% 🔴 |
| ↳ linter/@azure-tools/typespec-azure-core/no-rpc-path-params | 🟢 0.1ms | 🟢 0.2ms | +94.7% |
| ↳ linter/@azure-tools/typespec-azure-core/no-string-discriminator | 🟢 0.0ms | 🟢 0.0ms | +111.6% |
| ↳ linter/@azure-tools/typespec-azure-core/no-unknown | 🟢 0.1ms | 🟢 0.2ms | +97.7% |
| ↳ linter/@azure-tools/typespec-azure-core/no-unnamed-union | 🟢 0.2ms | 🟢 0.3ms | +88.0% |
| ↳ linter/@azure-tools/typespec-azure-core/operation-missing-api-version | 🟢 0.1ms | 🟢 0.2ms | +103.3% |
| ↳ linter/@azure-tools/typespec-azure-core/request-body-problem | 🟢 0.2ms | 🟢 0.3ms | +67.6% |
| ↳ linter/@azure-tools/typespec-azure-core/require-versioned | 🟢 0.0ms | 🟢 0.0ms | +37.3% |
| ↳ linter/@azure-tools/typespec-azure-core/response-schema-problem | 🟡 11.1ms | 🔴 22.1ms | +98.5% 🔴 |
| ↳ linter/@azure-tools/typespec-azure-core/rpc-operation-request-body | 🟢 0.2ms | 🟢 0.6ms | +292.6% |
| ↳ linter/@azure-tools/typespec-azure-core/spread-discriminated-model | 🟢 0.1ms | 🟢 0.3ms | +83.3% |
| ↳ linter/@azure-tools/typespec-azure-core/use-standard-names | 🟢 2.8ms | 🟢 5.4ms | +96.5% 🔴 |
| ↳ linter/@azure-tools/typespec-azure-core/use-standard-operations | 🟢 0.1ms | 🟢 0.1ms | +126.3% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-agent-base-type-child-resources | 🟢 1.9ms | 🟢 3.8ms | +101.0% 🔴 |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-agent-base-type-lifecycle-operations | 🟢 0.0ms | 🟢 0.0ms | +37.7% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-common-types-version | 🟢 1.9ms | 🟢 3.7ms | +93.8% 🔴 |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-custom-resource-no-key | 🟢 0.0ms | 🟢 0.1ms | +106.9% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-custom-resource-usage-discourage | 🟢 0.0ms | 🟢 0.1ms | +124.5% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-delete-operation-response-codes | 🟢 0.5ms | 🟢 1.1ms | +109.7% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-no-path-casing-conflicts | 🟢 2.2ms | 🟢 4.1ms | +91.3% 🔴 |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-no-record | 🟢 0.2ms | 🟢 0.4ms | +78.8% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-post-operation-response-codes | 🟢 0.2ms | 🟢 0.4ms | +92.2% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-put-operation-response-codes | 🟢 0.0ms | 🟢 0.0ms | +41.8% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-action-no-segment | 🟢 0.1ms | 🟢 0.2ms | +110.1% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-duplicate-property | 🟢 0.1ms | 🟢 0.1ms | +113.0% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-interface-requires-decorator | 🟢 0.0ms | 🟢 0.0ms | +85.6% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-action-verb | 🟢 0.0ms | 🟢 0.1ms | +98.1% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-envelope-property | 🟢 0.1ms | 🟢 0.1ms | +67.5% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-version-format | 🟢 0.0ms | 🟢 0.0ms | +63.5% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-key-invalid-chars | 🟢 0.1ms | 🟢 0.2ms | +87.7% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-name-pattern | 🟢 0.0ms | 🟢 0.0ms | +70.3% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-operation | 🟢 0.1ms | 🟢 0.2ms | +70.2% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-operation-response | 🟢 2.5ms | 🟢 4.8ms | +90.9% 🔴 |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-patch | 🟢 0.2ms | 🟢 0.3ms | +79.2% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-path-segment-invalid-chars | 🟢 0.1ms | 🟢 0.2ms | +78.6% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-provisioning-state | 🟢 0.1ms | 🟢 0.1ms | +83.9% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/beyond-nesting-levels | 🟢 0.0ms | 🟢 0.1ms | +94.1% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/empty-updateable-properties | 🟢 0.1ms | 🟢 0.1ms | +100.2% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/improper-subscription-list-operation | 🟢 0.0ms | 🟢 0.0ms | +55.4% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/lro-location-header | 🟢 7.0ms | 🟡 12.1ms | +72.0% 🔴 |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/missing-operations-endpoint | 🟢 0.0ms | 🟢 0.0ms | +53.9% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/missing-x-ms-identifiers | 🟢 0.2ms | 🟢 0.3ms | +80.9% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/no-empty-model | 🟢 0.1ms | 🟢 0.1ms | +88.6% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/no-override-props | 🟢 0.0ms | 🟢 0.1ms | +63.8% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/no-resource-delete-operation | 🟢 0.1ms | 🟢 0.2ms | +109.7% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/no-response-body | 🟡 11.3ms | 🟡 18.7ms | +65.7% 🔴 |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/patch-envelope | 🟢 0.1ms | 🟢 0.1ms | +105.5% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/resource-name | 🟢 0.1ms | 🟢 0.1ms | +102.8% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/secret-prop | 🟢 1.5ms | 🟢 2.7ms | +78.8% 🔴 |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/unsupported-type | 🟢 0.2ms | 🟢 0.4ms | +83.6% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/version-progression | 🟢 0.0ms | 🟢 0.0ms | +58.2% |
| ↳ linter/@azure-tools/typespec-client-generator-core/property-name-conflict | 🟢 0.5ms | 🟢 1.1ms | +102.9% |
| ↳ linter/@azure-tools/typespec-client-generator-core/require-client-suffix | 🟢 0.1ms | 🟢 0.2ms | +58.9% |
| emit | 🔴 3.27s | 🔴 5.77s | +76.8% 🔴 |
| ↳ emit/@azure-tools/typespec-autorest | 🟢 96.4ms | 🟢 161.2ms | +67.2% 🔴 |
| ↳ emit/@azure-tools/typespec-python | 🔴 2.25s | 🔴 4.22s | +87.3% 🔴 |
| ↳ emit/@typespec/http-client-js | 🔴 751.5ms | 🔴 1.17s | +55.3% 🔴 |
| ↳ emit/@typespec/openapi3 | 🟢 88.9ms | 🟢 146.2ms | +64.4% 🔴 |
| ↳ emit/@typespec/openapi3/compute | 🟢 75.6ms | 🟢 126.3ms | +66.9% 🔴 |
| ↳ emit/@typespec/openapi3/write | 🟢 13.4ms | 🟢 19.4ms | +45.1% 🔴 |
Averaged across 3 specs (azure-arm-resource-manager, azure-core-dataplane, azure-full).
Threshold: changes > ±5% are highlighted.
🟢 Fast · 🟡 Moderate (stages >200ms, rules >10ms) · 🔴 Slow (stages >400ms, rules >20ms)
|
You can try these changes here
|
tadelesh
reviewed
Jun 22, 2026
timotheeguerin
approved these changes
Jun 22, 2026
When an override operation contains any parameter with @clientLocation, the override is an intentional customization. Other parameters without @path are just pass-throughs and should not trigger the mismatch warning. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
l0lawrence
pushed a commit
to l0lawrence/typespec-azure
that referenced
this pull request
Jun 23, 2026
…realized @path params (Azure#4684) The "Run unit tests" job failed because the `@override` `@path`-preservation check (added in Azure#4644) reported `override-parameters-mismatch` on the `typespec-ts` `overrideReservedkeywords.md` scenario, even though the override is valid. ## Root cause - The check used `isPathParam`, which only inspects the `@path` decorator on the type graph. - In the failing ARM operation, the `name` property inherits `@path`/`@key` from `SubscriptionActionScope` and is collapsed with the body `name` by `collectParams`, so it *looks* like a path parameter. - The resolved route (`/subscriptions/{subscriptionId}/.../checknameavailability`) has no `{name}` segment — `name` is a body property — so requiring the override to keep it as `@path` is a false positive that affects ARM overrides broadly. ## Changes - **`$override` (`decorators.ts`)**: resolve the operation's real HTTP route via `getHttpOperation` and build the set of parameters *realized* as path params. The `@path`-preservation check now uses this set instead of the raw decorator, falling back to `isPathParam` when no HTTP route can be resolved (e.g. non-HTTP operations). - **`getRealizedPathParamNames` helper**: returns realized path-param names, or `undefined` on failure. - **Tests**: added a regression case asserting no mismatch is reported when a `@path`-decorated parameter is not realized in the route. - **Changeset** added. ```ts const realizedPathParamNames = getRealizedPathParamNames(context.program, original); const originalIsRealizedPathParam = realizedPathParamNames ? realizedPathParamNames.has(originalParam.name) : isPathParam(context.program, originalParam); ``` --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: xirzec <639216+xirzec@users.noreply.github.com>
timotheeguerin
pushed a commit
to timotheeguerin/typespec-azure
that referenced
this pull request
Jun 30, 2026
## Problem The `@override` `@path`-preservation check (added in Azure#4644, reworked in Azure#4693) produced false `override-parameters-mismatch` diagnostics on real ARM specs. Three distinct issues: 1. **Azure#4693** switched parameter matching to name-based, which over-relaxed the check. 2. **Azure#4644** compared parameters by sorted **position**; when an override adds/reorders parameters the index misaligns, flagging params that already carry `@path` (e.g. botservice `channelName`). 3. Determining "is a path parameter" from the `@path` decorator in the **type graph** (`isPathParam`) is unreliable: a parameter can carry `@path` without being part of the realized route (e.g. an ARM key surfaced by a templated `ArmProviderActionSync`, as in cognitiveservices `calculateModelCapacity`), and conversely a `@path` nested inside a plain model or `@bodyRoot` IS realized. ## Fix - **Revert Azure#4693** to restore positional structural comparison of the parameter lists. - **Resolve the original operation's realized HTTP route** (`getHttpOperation`) to get its actual path parameters — the ground truth that handles both phantom `@path` (orphaned, not in route → not enforced) and nested `@path` (in route → enforced). - **Match the corresponding override parameter by name** (not position) and require it to be a path parameter (`isPathParam`). This removes the `finishType` workaround and the body-param heuristics entirely. ## Result - All previously-failing ARM specs compile: storage, security, recoveryservicesbackup, quota, policyinsights, iothub, cognitiveservices, botservice, apimanagement, frontdoor, databoxedge. Overrides that genuinely dropped `@path` on a realized path param add `@path` (correct); phantom cases (cognitiveservices `calculateModelCapacity` `name`) need no spec change. - `override.test.ts` 16/16; full TCGC suite 1330 pass. --------- Co-authored-by: tadelesh <chenjieshi@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #4257